-
Notifications
You must be signed in to change notification settings - Fork 58
Lower ion to rtio #2204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Lower ion to rtio #2204
Conversation
Co-authored-by: Mehrdad Malek <[email protected]>
Co-authored-by: Mehrdad Malek <[email protected]>
Co-authored-by: Mehrdad Malek <[email protected]>
Co-authored-by: Mehrdad Malek <[email protected]>
Co-authored-by: Mehrdad Malek <[email protected]>
Co-authored-by: Mehrdad Malek <[email protected]>
Co-authored-by: Mehrdad Malek <[email protected]>
…lyst into rniczh/add-rtio-dialect
Co-authored-by: Mehrdad Malek <[email protected]>
Co-authored-by: Mehrdad Malek <[email protected]>
…lyst into rniczh/lower-ion-to-rtio
| auto traceValueWithCallback(Value value, CallbackT &&callback) | ||
| { | ||
| WalkResult walkResult = WalkResult::advance(); | ||
| std::queue<Value> visited; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nitpicky suggestion: I usually prefer worklist instead of visited since we are popping from it.
| if constexpr (std::is_same_v<std::invoke_result_t<CallbackT, Value>, WalkResult>) { | ||
| if (callback(value).wasInterrupted()) { | ||
| walkResult = WalkResult::interrupt(); | ||
| continue; | ||
| } | ||
| } | ||
| else { | ||
| callback(value); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this? Seems like all the callback defined in this file always return a WalkResult?
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here again, I don't see why this is needed. Especially that the return value is not used anywhere. Would it make sense to return a LogicalResult to help exist gracefully and get rid of the llvm::reportFatalInternalError?
| continue; | ||
| } | ||
| parentOp->emitError("Unsupported parent operation for block argument: ") << value; | ||
| llvm::reportFatalInternalError("Unsupported block argument"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a bit aggressive since it would abort compilation. I would rather exit gracefully if possible.
Context:
Convert ion dialect to rtio dialect
Description of the Change:
There are several steps to convert the ion dialect to rtio dialect:
Prepare the enviorment:
qreg allocandqreg extracttomemref. In the later conversion, we use the mapping to construct the channel forrtio.pulse. It will create a global memref that contains index from 0 to N to represent the qubits.parallel protocol to rtio:
Converts
ion.parallelprotocolregions into flat RTIO operations with proper synchronization. We rely on the data dependency of qubit to introduce the unrealized conversion cast betweenion.qubitandrtio.event. That's the way how we currently build the relationship between events. Later, those unrealized conversion cast will be removed via event propagation.Input mlir:
ion.pulsetortio.pulse:Converts
ion.pulsetortio.pulsewith calculated frequency.Frequency Calculation:
frequency = (reference_energy + detuning - raman1_frequency) / (2 * pi)Steps:
rtio.channelviartio.qubit_to_channelrtio.pulseoperationResolve channel mapping
Resolves dynamic channel references to static channel IDs.
propagate events
Resolves
unrealized_conversion_castand connect the ops with event typeSteps:
quantum.AllocOp: creatertio.emptyRemove all other dialects:
Removes unused quantum/ion/memref/linalg dialect after conversion.
TODOs:
Benefits:
Possible Drawbacks:
Related GitHub Issues:
[sc-100851]